Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix instances of: c++11 range-loop might detach Qt container warnings #5089

Merged
merged 2 commits into from
Nov 18, 2022
Merged

fix instances of: c++11 range-loop might detach Qt container warnings #5089

merged 2 commits into from
Nov 18, 2022

Conversation

mgallien
Copy link
Collaborator

analyzed via clazy

Signed-off-by: Matthieu Gallien matthieu.gallien@nextcloud.com

claucambra
claucambra reviewed Oct 24, 2022
View reviewed changes
Copy link
Collaborator

@claucambra claucambra left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some small comments that might be out of scope, but thought they were worth mentioning

src/3rdparty/kirigami/wheelhandler.cpp Outdated Show resolved Hide resolved
src/gui/folderman.cpp Show resolved Hide resolved
src/gui/folderman.cpp Show resolved Hide resolved
src/gui/folderman.cpp Show resolved Hide resolved
src/gui/folderman.cpp Outdated Show resolved Hide resolved
src/gui/folderman.cpp Outdated Show resolved Hide resolved
src/gui/tray/usermodel.cpp Show resolved Hide resolved
test/testallfilesdeleted.cpp Outdated Show resolved Hide resolved
test/testallfilesdeleted.cpp Outdated Show resolved Hide resolved
test/testallfilesdeleted.cpp Show resolved Hide resolved
allexzander
allexzander approved these changes Oct 29, 2022
View reviewed changes
Copy link
Contributor

@allexzander allexzander left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved, but, kindly see if you can fix our nitpicks on naming.

test/testallfilesdeleted.cpp Outdated Show resolved Hide resolved
test/testsyncconflict.cpp Outdated Show resolved Hide resolved
@mgallien mgallien force-pushed the ci/clazyRangeLoopDetach branch 3 times, most recently from af55075 to 9abec54 Compare November 15, 2022 17:33
@codecov
Copy link

codecov bot commented Nov 15, 2022 •
edited
Loading

Codecov Report

Merging #5089 (26fd4d2) into master (4135b75) will not change coverage.
The diff coverage is 75.00%.

Additional details and impacted files 
@@           Coverage Diff           @@
##           master    #5089   +/-   ##
=======================================
  Coverage   57.71%   57.71%           
=======================================
  Files         138      138           
  Lines       17441    17441           
=======================================
  Hits        10066    10066           
  Misses       7375     7375
Impacted Files Coverage Δ
src/libsync/creds/httpcredentials.cpp 6.91% <0.00%> (ø)
src/libsync/discovery.cpp 88.24% <100.00%> (ø)
src/libsync/syncengine.cpp 83.33% <100.00%> (ø)

@mgallien
fix instances of: c++11 range-loop might detach Qt container warnings
b5adbf3 
analyzed via clazy

Signed-off-by: Matthieu Gallien <matthieu.gallien@nextcloud.com>
@mgallien
tidy some loop related code: avoid one letter variable names
26fd4d2 
Signed-off-by: Matthieu Gallien <matthieu.gallien@nextcloud.com>
@mgallien mgallien force-pushed the ci/clazyRangeLoopDetach branch from 9abec54 to 26fd4d2 Compare November 18, 2022 15:02
@sonarcloud
Copy link

sonarcloud bot commented Nov 18, 2022

SonarCloud Quality Gate failed.    Quality Gate failed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 10 Code Smells

9.7% 9.7% Coverage
0.0% 0.0% Duplication

@nextcloud-desktop-bot
Copy link

AppImage file: nextcloud-PR-5089-26fd4d222678015515439da5ba5c41d5f8e0d6c9-x86_64.AppImage

To test this change/fix you can simply download above AppImage file and test it.

Please make sure to quit your existing Nextcloud app and backup your data.

@mgallien mgallien merged commit 1613056 into master Nov 18, 2022
@mgallien mgallien deleted the ci/clazyRangeLoopDetach branch November 18, 2022 17:16
@mgallien mgallien added this to the 3.7.0 milestone Nov 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@claucambra claucambra claucambra left review comments

@allexzander allexzander allexzander approved these changes

@camilasan camilasan Awaiting requested review from camilasan

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
3.7.0
Development

Successfully merging this pull request may close these issues.
4 participants
@mgallien @nextcloud-desktop-bot @allexzander @claucambra